Skip to content

[GitHub Action] Automatically open a PR for accepted guideline issues #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Jul 29, 2025

Conversation

x0rw
Copy link
Contributor

@x0rw x0rw commented May 31, 2025

Hey 👋.
This PR adds a workflow that automatically opens a PR for the accepted guidelines marked by 'accepted' label,
The script extracts the marked issue and parses it into a proper guideline rst file and appends this content into the appropriate chapter(eg. if the chapter is concurrency it will append the gl to src/coding-guidelines/concurreny.rst, note: the chapters title in here should be directly mapped to the names in coding-guidelines/ directory).

Tasks:

How to test? :

  • Merge my branch into your forked main
  • Enable issues from your fork's settings.
  • Create an issue with the guideline template.
  • Add 'status: approved' label (create the label first).
  • Now check Actions tab to see a 'auto-pr' job running.
  • After a while there should be a new PR by GA.
  • For iterative testing, remove and reassign the 'sign-off: create pr from issue' label(make sure it's removed and added from the issue's timeline) and GA will force push again on the same PR.

Current Limitations

Markdown headings (e.g., ##) cannot be used inside content blocks in the issue template that are converted to reStructuredText (.rst) directives.
Use bold text or other alternatives for emphasis or headings within these blocks.

Why?

  • Because .rst format doesn't allow having headings nested inside directives.

Copy link

netlify bot commented May 31, 2025

Deploy Preview for scrc-coding-guidelines ready!

Name Link
🔨 Latest commit ceae2f7
🔍 Latest deploy log https://app.netlify.com/projects/scrc-coding-guidelines/deploys/6886bf05e4ef8400084cedca
😎 Deploy Preview https://deploy-preview-122--scrc-coding-guidelines.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Collaborator

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @x0rw thanks for taking this on. I haven't reviewed it in detail yet.
Noticed that you could use an existing label tho 👍

By the way, I see the build is failing -- is that intentional / expected?
I'd like to start reviewing once the build passes.

@x0rw
Copy link
Contributor Author

x0rw commented Jun 7, 2025

Hi @PLeVasseur.
The build failed because of the FLS difference checks. I guess it was fixed in a previous PR. You can re-run it, and it should work fine.

@x0rw
Copy link
Contributor Author

x0rw commented Jun 7, 2025

Also, let me know when to remove those json test issues because i picked them randomly from my repo.

@x0rw x0rw requested a review from PLeVasseur June 11, 2025 22:48
@PLeVasseur
Copy link
Collaborator

Hey @x0rw -- could you rebase? @darkwisebear recently update the spec lock file.

@x0rw x0rw force-pushed the github-action/auto-pr branch 2 times, most recently from 34a4740 to 2d60b33 Compare July 12, 2025 14:02
@PLeVasseur
Copy link
Collaborator

Could you help me become less ignorant? 😅

What's going on here?
image

Is it possible to enable anyone to try to auto-rebase?

Is it this the Automatic Rebase action or something else? (that action looks kinda nice...)

@x0rw
Copy link
Contributor Author

x0rw commented Jul 12, 2025

What's going on here?

Is it possible to enable anyone to try to auto-rebase?

Is it this the Automatic Rebase action or something else? (that action looks kinda nice...)

Hey @PLeVasseur 😄
My local branch name is called github-action/auto-pr, i think that's what confused you.
When i rebase i simply do
git pull upstream main --rebase && git push origin "repo name" --force-with-lease,
but actually i think adding that Automatic Rebase /rebase command would be nice in here.

@x0rw
Copy link
Contributor Author

x0rw commented Jul 12, 2025

@PLeVasseur,
This is rebased, and i squashed some of those testing commits.

Copy link
Collaborator

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @x0rw -- sorry for the long delay 😅 I'm back from my "workation" in Japan and jumping back into reviews.

Bottom-line: I really like the concept and how you executed it.

Summary of feedback: As you can see from some of the comments I left, there's some refactoring that needs to be done to allow for easier maintenance in the future between both generate-guideline-templates.py and auto-pr-helper.py. Hope the rationale makes sense! 🙏

@x0rw
Copy link
Contributor Author

x0rw commented Jul 12, 2025

About testing the auto-pr script, I went with snapshot testing approach, i compare the generated .rst template from issue json with the expected .rst result,
I added the correspondant test workflow that only triggers when you change relevant files/dirs.
About the test_issue_01/02,json i pulled them from my own issues(not personal issues) for testing, we can change them to issues from this repo and have them snapshotted.

Copy link
Collaborator

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking on the refactoring @x0rw! I've suggested some bits here that I think could help. Could you take a look?

@PLeVasseur
Copy link
Collaborator

Hi @x0rw -- could you rebase? I think this looks solid enough to merge then. 👍

x0rw and others added 13 commits July 26, 2025 19:15
test auto-pr template 2

test auto-pr template 3

test auto-pr template 4

test auto-pr template 5

test auto-pr template 6

test auto-pr template 7

test auto-pr template 8

test auto-pr template 9
Fix 2

Fix 3

fix : special char i yml
add snapshot testing and their runner
renamed generate-guideline-templates.py to generate_guideline_templates.py
general refactor
Co-authored-by: Pete LeVasseur <[email protected]>
This workflow only triggers when the auto-pr script or the snapshot test directory changes
Co-authored-by: Pete LeVasseur <[email protected]>
@x0rw
Copy link
Contributor Author

x0rw commented Jul 26, 2025

Hey @PLeVasseur, I'm testing ways to convert markdown directly into .rst format so that issue #156 can be transformed, with links, tables, highlights, bullet points without compromising readability in the issue form.
So tomorrow we can get this merged.

@x0rw x0rw force-pushed the github-action/auto-pr branch from c1ebb63 to 9641978 Compare July 27, 2025 23:18
@x0rw
Copy link
Contributor Author

x0rw commented Jul 28, 2025

Hi @PLeVasseur,
This is ready for a live test, also check the Current Limitations section on this PR's description i just updated.

Copy link
Collaborator

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the contribution @x0rw ❤️

You could tackle this enhancement in another issue/PR:
it'd be nice to be able to support translating some syntax in the issue to a link to core/std as documented here.

@PLeVasseur PLeVasseur added this pull request to the merge queue Jul 29, 2025
Merged via the queue into rustfoundation:main with commit 22aac67 Jul 29, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants